Skip to content

Conversation

Renegade334
Copy link
Contributor

@Renegade334 Renegade334 commented Oct 8, 2025

Fixes #1641.
Fixes #2172.
Alternative to #1762.
Supersedes #2075.

Reference: https://webidl.spec.whatwg.org/#dfn-class-string

This amends the builder to append Symbol.toStringTag properties to interfaces and iterators in the ts6.0 core libraries. (ES6's well-known symbols are universally defined in ts6.0.)

Because adding a property with a literal type affects inheritance chains, only "final" classes (ie. those that do not have any inheritance children within the current global) can have these properties typed as string literals. (For example, it's not possible for Blob#[Symbol.toStringTag] to be typed as "Blob", because File inherits from Blob, and a property of type "File" can't override a parent property of type "Blob".)

For those interfaces that have one or more inheritance children, there are two possible approaches.

  • One is not to emit the property at all – but it's inconsistent to have some interfaces with Symbol.toStringTag properties, and others (including some very common ones) without them.
  • The alternative is to emit properties for these as well, but type them permissively as string, so that they can be correctly overridden by child interfaces. This is the approach taken here.

The toStringTag emitter does not add to any interface marked noInterfaceObject, since these are usually pseudo-types that are not true IDL interfaces, nor to namespaces converted to interfaces (ie. console).

Also adds a noToStringTag interface property to explicitly suppress emitting a Symbol.toStringTag property. This is useful for interfaces known to be a common target for userland inheritance, for example DOMException, which would otherwise be emitted within audioworklet.

Copy link
Contributor

github-actions bot commented Oct 8, 2025

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

},
"DOMException": {
"extends": "Error"
"extends": "Error",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move DOMException to a .kdl file, maybe move it to dom.kdl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the existing override is really beyond the scope of this changeset; would be better suited to a follow-up PR.

Copy link
Contributor

@Bashamega Bashamega Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the existing override is really beyond the scope of this changeset; would be better suited to a follow-up PR.

Then remove the changes in patches.ts

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But why noToStringTag at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error doesn't have a toStringTag property in the TS libs, and DOMException is a common target for userland inheritance, so the concern is that this will do more harm than good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should test it in TS repo without noToStringTag - and if it gives some trouble then we should consider it. Right now it feels arbitrary.

(idk much about testing in TS repo, @jakebailey knows more)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's pretty easy to check these variants (though a TS team member has to send the PR as we auto close anyone else trying to send a PR updating these files due to people not understanding they shouldn't touch generated files in the repo 😄)

@Renegade334 Renegade334 marked this pull request as ready for review October 8, 2025 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants